-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Exhaustively handle parsed attributes in CheckAttr #140372
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Some changes occurred in compiler/rustc_passes/src/check_attr.rs Some changes occurred in compiler/rustc_attr_data_structures |
This comment has been minimized.
This comment has been minimized.
e88b142
to
2e609cd
Compare
Thanks @jieyouxu . I think I read the CI error wrong and that's why I added the <> 😆 |
r? compiler |
I'm sorry for the long wait, however I finally started getting time to review these so I took it back and will review today :) |
@@ -57,14 +57,6 @@ impl OptimizeAttr { | |||
} | |||
} | |||
|
|||
#[derive(Clone, Debug, Encodable, Decodable, HashStable_Generic, PrintAttribute)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add this back in soon, but that's ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to do that actually, I just thought there shouldn't be a bunch of dead code here at this time.
Some minor comments, but looks good overall |
@rustbot author |
Reminder, once the PR becomes ready for a review, use |
This comment has been minimized.
This comment has been minimized.
@rustbot ready |
This pr
DiagnosticAttribute
struct and variantAttributeKind
enumCheckAttrVisitor
is now exhaustive forAttributeKind::Parsed
.I did not thoroughly check that there's no duplicated logic between this pass and the attribute parsing but I think it's OK.
r? @jdonszelmann